Update group cards further #928
Conversation
This causes the group name to be rendered as a link to the group page. This still uses the default tooltip, which is (IMO) wrong for the links on the `/groups` page.
* If not provided, defaults to the former hard-coded tooltip value * Includes changes to all `tooltipText` to be provided at to the `ListResources` component, as that's the level that knows what the tooltip for all the card should be As noted in the `FIXME` comment: I'm not wild about the text I came up with for the groups page and would welcome a better phrasing.
|
Pushing this for preliminary feedback on this first bit; more to come. |
| @@ -115,6 +115,7 @@ class GroupsPage extends React.Component { | |||
|
|
|||
| <ListResources | |||
| resourceType="dataset" | |||
| defaultGroupLinks={true} | |||
There was a problem hiding this comment.
[crystal ball gazing -- not a request for changes]
Implicit in the original design is that card titles will become links by simply adding the card name to the current URL. It works well for the groups page (e.g. groups + blab), but I don't think it'll work for an individual groups page.¹ We'll probably just disable the links for those pages.
Also, for what it's worth, the ={true} can be dropped, if you prefer.
¹ Depends on how card titles are chosen of course, but the blab group will probably have a "229e" card and /groups/blab/229e is 404.
There was a problem hiding this comment.
Depends on how card titles are chosen of course, but the blab group will probably have a "229e" card and /groups/blab/229e is 404.
I would think on the individual groups page, the titles would not include groups/<group_name>. The resourceListingCallback would just return pathPrefix: "groups/<group_name>"? (Maybe I'm completely off here, still catching up on all the ListResources work)
| @@ -15,9 +15,11 @@ interface ResourceGroupHeaderProps { | |||
| isCollapsed: boolean | |||
| resourcesToShowWhenCollapsed: number | |||
| quickLinks: QuickLink[] | |||
| tooltipText: string | |||
There was a problem hiding this comment.
There are quite a few tooltips within the resources UI, so I'd consider a prop name such as defaultGroupLinksTooltip.
There was a problem hiding this comment.
This is not the entire text and gets added to later. I would call it something like groupTooltipPrefix.
| resourceListingCallback={resourceListingCallback}/> | ||
| resourceListingCallback={resourceListingCallback} | ||
| // FIXME I do not love this text string; suggestions welcome | ||
| tooltipText="Click to load the group page" |
There was a problem hiding this comment.
[suggestion]
"Click to view this group in isolation, including additional information specific to that group"
Won't look nice if you then tack on for ${group.groupDisplayName || group.groupName} but we could change the prop to be a function accepting the group name.
There was a problem hiding this comment.
The Nextstrain docs for Groups call it a "splash page", so we can follow that pattern here:
"Click to view the individual group splash page"
There was a problem hiding this comment.
Or something less wordy:
Go to splash page for blab
Visit splash page for blab
| @@ -15,9 +15,11 @@ interface ResourceGroupHeaderProps { | |||
| isCollapsed: boolean | |||
| resourcesToShowWhenCollapsed: number | |||
| quickLinks: QuickLink[] | |||
| tooltipText: string | |||
There was a problem hiding this comment.
This is not the entire text and gets added to later. I would call it something like groupTooltipPrefix.
| if (!tooltipText) tooltipText="Click to load the default (and most recent) analysis"; | ||
| const setModalResource = useContext(SetModalResourceContext); |
There was a problem hiding this comment.
Suggestion: don't set a default here. Make sure every usage of ListResources defines the right text based on the context.
| resourceListingCallback={resourceListingCallback}/> | ||
| resourceListingCallback={resourceListingCallback} | ||
| // FIXME I do not love this text string; suggestions welcome | ||
| tooltipText="Click to load the group page" |
There was a problem hiding this comment.
Or something less wordy:
Go to splash page for blab
Visit splash page for blab
Description of proposed changes
Related issue(s)
#870
Checklist